Refactor api.rs, using VmState on Vm #27

efenniht · 2019-08-17T19:39:03Z

#17, #20 의 일부입니다.

Closes Which data structures can we modify their layout? #21.
- C 코드 중 vcpu와 vm의 내부를 직접 접근하는 코드를 getter/setter로 바꾸었습니다.
- 이것을 확인하는 겸 해서 {cpu, vm}.h 에는 forward declaration만 남겨두었습니다.
- offset.c 는 상수로 정의된 각 필드의 오프셋이 실제로 맞는 지 테스트하는 코드인데, 이것은 aarch64.rs 에 옮겼습니다.
  - Filename convention #15 를 해결할 때 같이 수정해야 합니다.
  - 사실 가장 좋은 것은 빌드스크립트를 만들어서 자동으로 헤더를 생성하는 건데... 나중으로 미루는 걸로...
Vm 중 락으로 보호해야 하는 데이터를 VmState로 추려서 SpinLock으로 보호했습니다.
- "락으로 보호해야 하는 데이터" 에는 wait_entries 가 포함됩니다. 이것을 손보는 것은 다음 기회에.
- "락으로 보호해야 하는 데이터" 에는 vcpus가 포함되지 않습니다. vcpus의 락으로 보호해야 하는 데이터는 VCpuState에 걸린 락으로 보호되기 때문입니다.
이 락을 사용하도록 api.rs 코드의 많은 부분을 변경했습니다.
- 하지만 아직 goto 문의 잔재는 남아있습니다. 페이지 테이블에 대한 트랜잭션? 같은 걸 만들어 볼까 생각 중입니다.
VmState 와 관련된 api_* 함수들을 VmState의 메서드로 옮겼습니다.
- 그러면서 public한 멤버 변수를 줄이기로 노력했는데, 깔끔하게 모두 없애지는 못했습니다. (Remove unneccessary pubs. #12)
- 예를 들어, api_mailbox_clear는 일견 Mailbox의 멤버 함수가 될 수 있을 것 같지만 나중에 api_switch_to_primary 를 내부적으로 호출해서 간단하게는 되지 않습니다.
- 나중에 좀 더 분석해서 API 파트와 VM 파트를 분리하는 걸로...
Vm 관련 함수들을 리팩토링하면서 unsafe를 제거하거나 필요한 부분을 explicit하게 나타냈습니다.
VmLocked 는 SpinLockGuard<VmState>를 저장하도록 바꾸고 싶었는데, C 코드에서 vm_locked 내부의 vm을 사용하는 부분이 있어서 당장은 하지 않았습니다. spci_architected_message.c 를 Rust로 먼저 포팅해야 가능할 것으로 보입니다.
- api 내부의 함수가 SpinLockGuard<VmState>를 가지고 있는 상태로 VmLocked을 인자로 받는 C 함수를 호출하는 게 문제인데요. 이 C 함수가 *mut VmLocked를 받도록 바꾸면 될 것 같지만 어차피 포팅할 코드이므로 나중에...

아직 #20 은 끝나지 않았습니다. 다음으로는 VCpu 에 있는 락을 수정할 예정입니다.

여담으로, 이 리팩토링을 하면서 Rust에서 mut 키워드가 가지는 의미에 대해서 좀 더 깊게 이해하게 되었습니다:

&mut은 mutability가 아니라 배타적인 레퍼런스를 의미합니다. 배타적인 레퍼런스는 안전한 수정 가능성을 의미하지만, 역은 성립하지 않고 Rust에서는 안전한 수정을 보장하는 다른 방법이 많습니다.
let mut x; 로 선언된 변수 또한 &mut 레퍼런스를 만들 수 있다는 것을 의미합니다. 이게 꼭 이 변수를 수정한다는 뜻은 아닙니다. 예를 들어 SpinLockGuard는 &mut self를 받아서 mutable한 내부 레퍼런스를 줍니다.

…etters.

…-- Now it passes all tests.

- Use SpinLock<VmState> - Move vm-related functions into vm.rs will be continued...

…_waiter`

…to `*const Vm`. Also holds for page pools

jeehoonkang

PR이 커서 일단 일부 코멘트만 남깁니다. 몇번 더 iteration을 돌고 머지하면 좋을 것 같습니다.

고생하셨습니다!

hfo2/src/arch/aarch64.rs

hfo2/src/cpu.rs

hfo2/src/dlog.rs

hfo2/src/vm.rs

… configure_stage1.

…prints error message for invalid string.

…t VmState)

jeehoonkang

일단 이정도만 남깁니다... 추후에 다시 리뷰 또 드리겠습니다.

hfo2/src/api.rs

hfo2/src/vm.rs

hfo2/src/arch/aarch64.rs

efenniht · 2019-08-20T07:18:02Z

모두 resolve 했습니다!

jeehoonkang · 2019-08-20T07:27:18Z

고생하셨어요-

efenniht added 8 commits August 16, 2019 17:46

Prevents C codes from accessing inner-struct fields. Instead, makes g…

9e930e5

…etters.

For consistent memory layout, preserves #[repr(C)] of VCpu. And typo …

7faac80

…-- Now it passes all tests.

Refactor api_debug_log.

86b96fc

Oxidize api.rs.

cf42465

- Use SpinLock<VmState> - Move vm-related functions into vm.rs will be continued...

Oxidize api.rs (cont'd)

deca8a1

Bug fix, by giving better name is_waiter_list_empty instead of `any…

fb88e1d

…_waiter`

Typo, and use is_null()

93f751c

Mutability of Vm is now guranteed by SpinLock, thus change *mut Vm …

a6a4089

…to `*const Vm`. Also holds for page pools

efenniht assigned efenniht and jeehoonkang Aug 18, 2019

jeehoonkang reviewed Aug 18, 2019

View reviewed changes

efenniht added 13 commits August 19, 2019 12:13

Oxidizes vcpu_handle_page_fault and reduces mut.

9e61ebc

Removes lock_hypervisor_table and rustfmt.

8e47489

Initializes Mailbox in the same order of fields, and removes mut on…

426ff49

… configure_stage1.

Use is_{none, some}() rather than if let Some(_) and if let None.

1e3e8fa

Refactors to use Option::filter rather than verbose if.

d015065

Early returns and remove unnecessary type hint

ab3e8d0

We don't need to struggle with vm's buffer.

32f9b4a

It is weird that Hafnium panics as the VM sends non-UTF8 bytes. Just …

b702cce

…prints error message for invalid string.

Leaves TODO.

d37feaa

Renames VmState to VmInner.

ff85cfc

Write a comment about publicity of stack_bottom.

7eeee87

Making a safe wrapper function to get the root of PageTable of Vm (no…

aa51119

…t VmState)

Add a kind comment

d82fe49

jeehoonkang reviewed Aug 19, 2019

View reviewed changes

hfo2/src/api.rs Outdated Show resolved Hide resolved

hfo2/src/api.rs Outdated Show resolved Hide resolved

hfo2/src/api.rs Show resolved Hide resolved

hfo2/src/vm.rs Show resolved Hide resolved

hfo2/src/arch/aarch64.rs Show resolved Hide resolved

efenniht added 4 commits August 20, 2019 10:52

Improve comments.

61c77e4

is_none().

479a241

TODO about spci_architected_message.c.

0749e40

Removes wrongly ported line (why did I write assert!(false)?)

b86e4d9

efenniht added 8 commits August 20, 2019 11:46

Comments about forward declaration of vcpu and vm.

020277a

Edits wrong comments.

e844cb9

Returns Result<(), ()> rather than Option<()>.

43f80ab

Comments about Result.

c6d48d3

Typo

1708acd

Use Result<(), ()> rather than Option<()>, in api.rs.

653ef12

Use inner for VmInner, rather than state.

bec726a

TODO about making a trait generalizing MPool

79cdabe

jeehoonkang merged commit c8cc29b into kaist-cp:hfo2 Aug 20, 2019

efenniht deleted the hfo2-17-api branch August 21, 2019 03:51

efenniht mentioned this pull request Aug 21, 2019

unsafe 양을 측정? #33

Closed

efenniht mentioned this pull request Aug 28, 2019

Race on dlog_flush_vm_buffer #28

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor api.rs, using VmState on Vm #27

Refactor api.rs, using VmState on Vm #27

efenniht commented Aug 17, 2019 •

edited

Loading

jeehoonkang left a comment

jeehoonkang left a comment

efenniht commented Aug 20, 2019

jeehoonkang commented Aug 20, 2019

Refactor api.rs, using VmState on Vm #27

Refactor api.rs, using VmState on Vm #27

Conversation

efenniht commented Aug 17, 2019 • edited Loading

jeehoonkang left a comment

Choose a reason for hiding this comment

jeehoonkang left a comment

Choose a reason for hiding this comment

efenniht commented Aug 20, 2019

jeehoonkang commented Aug 20, 2019

efenniht commented Aug 17, 2019 •

edited

Loading